feat: Notifications, visual fixes #2884
Conversation
StaNov
force-pushed
the
stanov/visual-fixes
branch
from
d3bc103 to
a9a51e7
Compare
StaNov
force-pushed
the
stanov/visual-fixes
branch
from
bfadc3e to
018e172
Compare
| .replaceAll(/\b(\d+?)\b ([Mm])inut.*?\b/g, '$1 $2in') | ||
| .replaceAll(/\b(\d+?)\b hodin.*?\b/g, '$1 hod') | ||
| .replaceAll(/\b(\d+?)\b (sekund|second|segund).*?\b/g, '$1 s'); | ||
| } |
There was a problem hiding this comment.
Hmm, this doesn't look very clean. Is it necessary? If we keep it there should be some comment about it.
There was a problem hiding this comment.
I hope this is some kind of joke. Can you please explain what it does?
There was a problem hiding this comment.
Anyway, this has to very probably have to go as separate translations with proper pluralization.
There was a problem hiding this comment.
Thanks for your comments, guys.
The reasoning is this:
- The requirement from design was to show the age of the notification.
- We are already using the
date-fnscomponent at other places, so I decided to use it also here. - After another design iteration, it was found that
15 minutes agois too long and it would be better to abbreviateminutesto justmin,secondstosandhourstoh. - The
date-fnscomponent doesn't offer a simple configuration for this (as far as I researched) besides of creating newdate-fns-locales for this case. It is supplied with two dates and a locale and it returns a localized string. - So my low-level effort to at least prototype it to see it on the screen was this - take the output string and shorten the long words to the abbreviated ones.
- After another design iteration, it might be better to drop using the localized text and go for just
15mor7d.
There was a problem hiding this comment.
I am for making it simplier...
It doesn't read well. Classic meme issue with reading row first.
I would remove all the not-necessary information.
- we don't need the language there (the user probably knows which languages they know)
- the user don't need to know task number
- the user probably doesn't need the project name, because there is usually single project
- Actually, instead of showing the user avatar, I would show the project avatar, if it's project event. I am not that interested about the information who assigned me. (usually it will be my single manager)
Anyway, they will find out when they open it.
The notifications should not replicate the function of activity log. Notifications only notify. It's role is not to inform about everything.
The time can be super short format. Like facebook and reddit have.
Reddit example:
Facebook example:
There was a problem hiding this comment.
I put away the confusing regex shortenings.
Could we please merge it as it is for now since the fixes are according to the current design?
| .replaceAll(/\b(\d+?)\b ([Mm])inut.*?\b/g, '$1 $2in') | ||
| .replaceAll(/\b(\d+?)\b hodin.*?\b/g, '$1 hod') | ||
| .replaceAll(/\b(\d+?)\b (sekund|second|segund).*?\b/g, '$1 s'); | ||
| } |
There was a problem hiding this comment.
I hope this is some kind of joke. Can you please explain what it does?
| .replaceAll(/\b(\d+?)\b ([Mm])inut.*?\b/g, '$1 $2in') | ||
| .replaceAll(/\b(\d+?)\b hodin.*?\b/g, '$1 hod') | ||
| .replaceAll(/\b(\d+?)\b (sekund|second|segund).*?\b/g, '$1 s'); | ||
| } |
There was a problem hiding this comment.
Anyway, this has to very probably have to go as separate translations with proper pluralization.
StaNov
force-pushed
the
stanov/visual-fixes
branch
from
018e172 to
d0c6178
Compare
StaNov
force-pushed
the
feature/notifications
branch
from
6d8b857 to
9e1f094
Compare
StaNov
force-pushed
the
stanov/visual-fixes
branch
from
d0c6178 to
8da1ad3
Compare
StaNov
force-pushed
the
stanov/visual-fixes
branch
from
8da1ad3 to
70d504e
Compare
No description provided.